Skip to content

🎨 added docker hub registry auth to director-v2 when creating services ⚠️ #8321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Sep 9, 2025
Merged

🎨 added docker hub registry auth to director-v2 when creating services ⚠️ #8321

merged 5 commits into from
Sep 9, 2025

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Sep 5, 2025 •
edited
Loading

What do these changes do?

Adds authentication when services are created via director-v2. This was missing and was causing images on Docker Hub to not be used.

Related issue/s

How to test

Dev-ops ⚠️

  • currently on all our deployments the DIRECTOR_V2_DOCKER_HUB_REGISTRY={} which means it loads the credentials for our internal registry. Please change this so it uses the appropriate settings @YuryHrytsuk

@GitHK GitHK self-assigned this Sep 5, 2025
@GitHK GitHK added the a:director-v2 issue related with the director-v2 service label Sep 5, 2025
@GitHK GitHK added this to the Cheops milestone Sep 5, 2025
@GitHK GitHK marked this pull request as ready for review September 5, 2025 12:44
@GitHK GitHK requested a review from Copilot September 5, 2025 12:56
Copilot AI reviewed Sep 5, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Docker Hub registry authentication to the director-v2 service creation process. The main purpose is to enable authentication when pulling Docker images from Docker Hub, which was previously missing and causing failures when trying to use images from Docker Hub.

  • Adds authentication support to the create_service_and_get_id function
  • Updates all test calls to include the new authentication parameter
  • Modifies the service creation logic in the scheduler to pass Docker Hub registry settings

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_api/_core.py Updates the core service creation function to accept and use Docker Hub registry authentication settings
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_event_create_sidecars.py Modifies the scheduler to pass app settings for Docker Hub authentication when creating services
services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_api.py Updates all test calls to include the new authentication parameter with None values

@codecov
Copy link

codecov bot commented Sep 5, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.77%. Comparing base (d443f36) to head (fd5255e).

❗ There is a different number of reports uploaded between BASE (d443f36) and HEAD (fd5255e). Click for more details.

HEAD has 30 uploads less than BASE
Flag BASE (d443f36) HEAD (fd5255e)
unittests 31 1
Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #8321       +/-   ##
===========================================
- Coverage   89.58%   68.77%   -20.81%     
===========================================
  Files        1744      760      -984     
  Lines       68230    34871    -33359     
  Branches      828      175      -653     
===========================================
- Hits        61124    23983    -37141     
- Misses       6886    10831     +3945     
+ Partials      220       57      -163
Flag Coverage Δ
integrationtests 64.08% <80.00%> (+0.01%) ⬆️
unittests 84.55% <30.00%> (-3.56%) ⬇️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 76.84% <ø> (-8.14%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 90.99% <80.00%> (-0.07%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 81.87% <ø> (-8.59%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.96% <ø> (-29.04%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d443f36...fd5255e. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Sep 5, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for fd5255e.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI unit-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@GitHK GitHK requested a review from pcrespov September 5, 2025 13:12
YuryHrytsuk
YuryHrytsuk approved these changes Sep 5, 2025
View reviewed changes
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

sanderegg
sanderegg approved these changes Sep 5, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@YuryHrytsuk YuryHrytsuk mentioned this pull request Sep 5, 2025
Closed
14 tasks
@GitHK GitHK enabled auto-merge (squash) September 8, 2025 11:47
@GitHK GitHK changed the title 🎨 added docker hub registry auth to director-v2 when creating services 🎨 added docker hub registry auth to director-v2 when creating services ⚠️ Sep 8, 2025
@GitHK GitHK added the t:maintenance Some planned maintenance work label Sep 8, 2025
bisgaard-itis
bisgaard-itis approved these changes Sep 9, 2025
View reviewed changes
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍🏻

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@GitHK GitHK merged commit 8a512a9 into ITISFoundation:master Sep 9, 2025
92 of 95 checks passed
@GitHK GitHK deleted the pr-osparc-login-director-v2 branch September 9, 2025 06:41
@YuryHrytsuk YuryHrytsuk mentioned this pull request Sep 11, 2025
Closed
3 tasks
@pcrespov pcrespov mentioned this pull request Sep 11, 2025
Closed
16 tasks
This was referenced Sep 18, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@pcrespov pcrespov pcrespov approved these changes

@sanderegg sanderegg sanderegg approved these changes

@YuryHrytsuk YuryHrytsuk YuryHrytsuk approved these changes

@bisgaard-itis bisgaard-itis bisgaard-itis approved these changes

@giancarloromeo giancarloromeo Awaiting requested review from giancarloromeo

@matusdrobuliak66 matusdrobuliak66 Awaiting requested review from matusdrobuliak66

Assignees

@GitHK GitHK

Labels

a:director-v2 issue related with the director-v2 service t:maintenance Some planned maintenance work

Projects

None yet

Milestone

Cheops

Development

Successfully merging this pull request may close these issues.

5 participants

@GitHK @pcrespov @sanderegg @YuryHrytsuk @bisgaard-itis